Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add PULUMI_STACK for compiler run enviroment #578

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

phramz
Copy link
Contributor

@phramz phramz commented Jun 3, 2024

This PR adds a new environment variable PULUMI_STACK to the compiler run.

The possibility to determine the stack from the compiler makes it a lot more flexibel when dealing with stack specific resources.

Here an example (I'm using Jinja2 though the issue itself is not compiler specific):

# Pulumi.yaml
runtime:
  name: yaml
  options:
    compiler: jinja2 --format=yaml Pulumi.yaml values.yaml
    
 resources:
  # stack specific resources for stage
  # {% if 'stage' == 'PULUMI_STACK'|getenv %}{% include 'templates/mailcatcher.yaml.j2' %}{% endif %}
  
  # common resources

A more complex scenario would be to add HA resources to a production stack where an integration system is zonal set up to reduce cloud costs.

My approach seems a bit naive to be honest, but I couldn't figure out a better way without breaking interfaces. Also this solution only works from the Run-call context, so GetRequiredPlugins might not be accurate when plugins are stack specific.

WDYT?

@phramz phramz requested a review from a team as a code owner June 3, 2024 11:41
Copy link

github-actions bot commented Jun 3, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

Copy link
Contributor

@tgummerer tgummerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! This seems reasonable to me at a glance. Ideally we would also have a test showing that this works.

Also curious if others from @pulumi/platform or @AaronFriel have any opinion on this, or other suggestions of how to implement this.

pkg/pulumiyaml/run.go Outdated Show resolved Hide resolved
pkg/pulumiyaml/run.go Outdated Show resolved Hide resolved
pkg/server/server.go Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jun 3, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

@phramz
Copy link
Contributor Author

phramz commented Jun 3, 2024

Ideally we would also have a test showing that this works.

Regarding a test ... I'm not sure where and how to ideally implement such a test. Would be an acceptance test unless we can mock the exec package somehow (probably a little refactoring).

Any thoughts or ideas on this @tgummerer

@tgummerer
Copy link
Contributor

I think an integration test would be fine here, maybe creating two different stacks with different outputs based on the environment variables you are introducing here? pkg/tests/integration_test.go has some examples of current integration tests.

CHANGELOG_PENDING.md Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jun 3, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

@phramz
Copy link
Contributor Author

phramz commented Jun 3, 2024

I think an integration test would be fine here, maybe creating two different stacks with different outputs based on the environment variables you are introducing here? pkg/tests/integration_test.go has some examples of current integration tests.

@tgummerer I'm having a hard time setting up the integration tests to run locally. Obviously Pulumi is not using my updated binary as language plugin. So my changes don't have any effect during the test. Is there a doc or could you give me a hint on how to run them locally.

I already pushed an unfinished test PTAL.

Installing the binary will give me an error:

error: the yaml language plugin is bundled with Pulumi, and cannot be directly installed with this command. If you need to reinstall this plugin, reinstall Pulumi via your package manager or install script.

pkg/pulumiyaml/run.go Outdated Show resolved Hide resolved
@tgummerer
Copy link
Contributor

/run-acceptance-tests

Copy link

github-actions bot commented Jun 4, 2024

Please view the results of the PR Build + Acceptance Tests Run Here

@tgummerer
Copy link
Contributor

Installing the binary will give me an error:

How exactly were you trying to install the binary? I believe you should be able to run make install to install the binary (and then prepend $GOBIN to the $PATH.) There's also a make test Makefile target that should run all tests. Does that not work for you?

Copy link

github-actions bot commented Jun 4, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

@phramz
Copy link
Contributor Author

phramz commented Jun 4, 2024

Installing the binary will give me an error:

How exactly were you trying to install the binary? I believe you should be able to run make install to install the binary (and then prepend $GOBIN to the $PATH.) There's also a make test Makefile target that should run all tests. Does that not work for you?

Thanks! It was indeed a PATH issue having both Pulumi from source and Brew in place.

I finished/fixed the test, fingers crossed it'll pass now.

Copy link

github-actions bot commented Jun 4, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

@Frassle
Copy link
Member

Frassle commented Jun 4, 2024

/run-acceptance-tests

Copy link

github-actions bot commented Jun 4, 2024

Please view the results of the PR Build + Acceptance Tests Run Here

Copy link

github-actions bot commented Jun 4, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

@tgummerer
Copy link
Contributor

/run-acceptance-tests

Copy link

github-actions bot commented Jun 4, 2024

Please view the results of the PR Build + Acceptance Tests Run Here

Copy link

github-actions bot commented Jun 4, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

@tgummerer
Copy link
Contributor

/run-acceptance-tests

Copy link

github-actions bot commented Jun 4, 2024

Please view the results of the PR Build + Acceptance Tests Run Here

pkg/pulumiyaml/run.go Outdated Show resolved Hide resolved
pkg/server/server.go Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jun 4, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

…d `PULUMI_CONFIG` as environment variable to compiler process
Copy link

github-actions bot commented Jun 4, 2024

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project

@tgummerer
Copy link
Contributor

/run-acceptance-tests

Copy link

github-actions bot commented Jun 4, 2024

Please view the results of the PR Build + Acceptance Tests Run Here

Copy link
Contributor

@tgummerer tgummerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you again, and thanks for the patience while we worked out the best way to also make this backwards compatible.

I'm gonna leave this PR open until tomorrow just in case anyone else has any comments, but this looks good to me!

@phramz
Copy link
Contributor Author

phramz commented Jun 4, 2024

Thank you again, and thanks for the patience while we worked out the best way to also make this backwards compatible.

I'm gonna leave this PR open until tomorrow just in case anyone else has any comments, but this looks good to me!

That went really smooth, kudos and big thanks to you!

@tgummerer tgummerer merged commit aed8f2d into pulumi:main Jun 5, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants